Skip to content

fix(noema): replay OIDC envelope repair on current main - #834

Open
seonghobae wants to merge 5 commits into
mainfrom
fix/noema-oidc-envelope-current-main-20260808
Open

fix(noema): replay OIDC envelope repair on current main#834
seonghobae wants to merge 5 commits into
mainfrom
fix/noema-oidc-envelope-current-main-20260808

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

RCA

The Noema OIDC exchange can successfully return its stable success envelope while the central consumer still fails because protected main reads the nonexistent top-level .token. The actual token is repository-bound under data.token; treating the resulting empty lookup as a missing credential misdiagnoses a producer/consumer schema mismatch.

Historical PR #822 contains the bounded repair but is no longer mergeable and is based on protected-main 1131b1bbafb24e455fc8619cdf316813e8721861. Protected main advanced to 6eb06cdd08c79a06f7b390069d4ffa49e2eb7dba. Checks, reviews, approvals, comments, and synthetic merge evidence from #822 do not transfer here.

Feasibility decision

A destructive rebase/force-push is unnecessary. The old base → current main delta touches only the Strix quality workflow, its doctoring/test, and one independent CHANGELOG.md entry; it does not modify noema-review.yml. This replacement is therefore rebuilt directly from exact current protected main and carries only the same four-file bounded OIDC contract, while preserving the intervening Strix changelog entry.

Bounded repair

  • require ok: true and object-valued data;
  • require non-empty data.token, exact target repository identity, and non-empty workflow-source/expiry metadata;
  • extract only .data.token after the full envelope validates;
  • mask the token immediately and never reflect the raw response;
  • preserve GitHub App/PAT fallbacks, reviewer identities, token names/scopes, LLM/model routing, NVIDIA behavior, permissions, branch protection, merge/release authority, and deployment authority;
  • retain the focused regression contract and APA 7 doctoring.

Exact replacement boundary

This branch is exactly four commits ahead and zero behind protected main at creation. The effective diff is limited to:

  • .github/workflows/noema-review.yml
  • tests/test_noema_oidc_exchange_contract.py
  • docs/doctoring/noema-oidc-exchange-envelope.md
  • CHANGELOG.md

No predecessor evidence transfers. Keep Draft until implementation/tests/docs/changelog are complete and every currently executable exact-head deterministic/security/supply-chain gate is green with zero current source-backed findings. At that point Ready status is permitted solely to obtain current-head automated and independent review; Ready is not merge evidence. Merge remains prohibited until current automated review is complete, zero valid unresolved findings remain, a qualifying non-author counted approval exists, and branch protection/repository policy allow the exact unchanged head.

Operational acceptance

After protected integration, execute a real Noema OIDC exchange and prove the stable response envelope reaches token extraction successfully without exposing the token. PR checks alone are not operational closure.

Supersedes #822 only after this replacement proves its own exact-head gates.

Summary by CodeRabbit

  • 버그 수정

    • OIDC 토큰 교환 응답을 엄격히 검증하여 잘못되거나 저장소 정보가 일치하지 않는 응답을 안전하게 거부합니다.
    • 표준 응답 형식에서 토큰을 추출하고, 진단 출력에 민감한 토큰이 노출되지 않도록 개선했습니다.
  • 문서

    • OIDC 응답 형식, 검증 기준, 토큰 보호 및 호환성 원칙을 문서화했습니다.
  • 테스트

    • 정상 응답 처리, 오류 검증, 토큰 마스킹 및 출력 저장 동작에 대한 회귀 테스트를 추가했습니다.

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d1f65907-135b-45ae-8bd4-8b4137492b4d

📥 Commits

Reviewing files that changed from the base of the PR and between 97276da and 7e3c9e5.

📒 Files selected for processing (1)
  • tests/test_noema_oidc_exchange_contract.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/test_noema_oidc_exchange_contract.py

📝 Walkthrough

Walkthrough

Noema OIDC 교환 응답에 fail-closed 검증을 추가했습니다. 워크플로는 저장소에 바인딩된 data.token만 추출합니다. 테스트와 문서는 응답 계약, 오류 처리, 토큰 비노출을 검증합니다.

Changes

Noema OIDC 응답 검증

Layer / File(s) Summary
OIDC 응답 envelope 검증
.github/workflows/noema-review.yml, docs/doctoring/noema-oidc-exchange-envelope.md
워크플로가 성공 상태, data 객체, 저장소 일치 여부, workflow reference, 만료 시각과 비어 있지 않은 data.token을 검증합니다. 응답 계약과 토큰 비노출 규칙을 문서화했습니다.
계약 회귀 검증
tests/test_noema_oidc_exchange_contract.py, CHANGELOG.md
표준 성공 응답 소비, 잘못된 응답 오류 처리, 토큰 마스킹과 GitHub Actions 출력 기록을 검증합니다. 변경 사항을 changelog에 기록했습니다.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant NoemaOIDC
  participant GitHubWorkflow
  participant jq
  NoemaOIDC->>GitHubWorkflow: OIDC 교환 응답 반환
  GitHubWorkflow->>jq: 응답 envelope 및 저장소 메타데이터 검증
  jq-->>GitHubWorkflow: 검증 결과와 data.token 반환
Loading

Possibly related PRs

Suggested reviewers: opencode-agent

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 Noema OIDC 응답 봉투 수정이라는 주요 변경 사항을 명확하게 요약합니다.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/noema-oidc-envelope-current-main-20260808

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review the unchanged exact head 97276da73c3b5d7312ce01eb88f8260ff75f3930 against protected-base 6eb06cdd08c79a06f7b390069d4ffa49e2eb7dba. Focus on the bounded four-file Noema OIDC response-envelope repair; do not infer evidence from predecessor #822.

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

@seonghobae I will review exact head 97276da73c3b5d7312ce01eb88f8260ff75f3930 against protected-base 6eb06cdd08c79a06f7b390069d4ffa49e2eb7dba. I will limit the review to the four-file Noema OIDC response-envelope repair. I will not use evidence from #822.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Re-review only the unchanged exact head 97276da73c3b5d7312ce01eb88f8260ff75f3930 against protected base 6eb06cdd08c79a06f7b390069d4ffa49e2eb7dba. The previous request was rate-limited several hours ago. Review only the bounded four-file Noema OIDC success-envelope repair and do not reuse evidence from superseded #822.

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

@seonghobae I will review exact head 97276da73c3b5d7312ce01eb88f8260ff75f3930 against protected base 6eb06cdd08c79a06f7b390069d4ffa49e2eb7dba.

I will limit the review to the four-file Noema OIDC response-envelope repair. I will not use evidence from superseded PR #822.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Re-review only unchanged exact head 97276da73c3b5d7312ce01eb88f8260ff75f3930 against protected base 6eb06cdd08c79a06f7b390069d4ffa49e2eb7dba. All exposed exact-head machine gates are terminal-success and there are zero review threads. Prior review attempts were capacity-limited; this request is sent only after the published cooldown window elapsed. Limit review to the four-file Noema OIDC success-envelope repair and do not use predecessor #822 evidence.

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

@seonghobae I will review exact head 97276da73c3b5d7312ce01eb88f8260ff75f3930 against protected base 6eb06cdd08c79a06f7b390069d4ffa49e2eb7dba.

I will limit the review to the four-file Noema OIDC success-envelope repair. I will not use evidence from superseded PR #822.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/test_noema_oidc_exchange_contract.py`:
- Around line 41-44: Update the assertions in the exchange contract test to
verify that the `echo "::add-mask::$app_token"` command appears before `echo
"token=$app_token" >>"$GITHUB_OUTPUT"` within `exchange`, while preserving the
existing presence checks.
- Line 24: Update the assertion in the noema OIDC exchange contract test so it
rejects the `.token // empty` jq pattern regardless of whether the filter uses
single or double quotes; prefer checking the quote-independent pattern itself.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8cc60992-3a83-462d-b8da-6169c065e230

📥 Commits

Reviewing files that changed from the base of the PR and between 6eb06cd and 97276da.

📒 Files selected for processing (4)
  • .github/workflows/noema-review.yml
  • CHANGELOG.md
  • docs/doctoring/noema-oidc-exchange-envelope.md
  • tests/test_noema_oidc_exchange_contract.py

Comment thread tests/test_noema_oidc_exchange_contract.py Outdated
Comment thread tests/test_noema_oidc_exchange_contract.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant